tidy: add unit tests for alphabetical checks #117149

nnethercote · 2023-10-25T00:11:40Z

I discovered there aren't any tests while working on #117068.

Currently, if a `tidy-alphabetical-end` marker appears on the last line of a file, tidy will erroneously issue a "reach end of file expecting `tidy-alphabetical-end`" error. This is because it uses `take_while` within `check_section`, which consumes the line with the end marker, and then after `check_section` returns `check` peeks for at least one more line, which won't be there is the marker was on the last line. This commit fixes the problem, by removing the use of `take_while`, and doing the "reached end of file" test within `check_section` without using `peek`. It also renames `{START,END}_COMMENT` as `{START,END}_MARKER`, which is a more appropriate name.

These are comment lines in `Cargo.toml` files. But exclude lines starting with `#!` from the skipping, because we want to check them. (Rust `#![feature(...)]` lines.) Also allow empty lines, which are occasionally useful.

- Tweak some comments. - No need to do the `concat!` trick on `START_MARKER` because it's immediately followed by `END_MARKER`. - Fix an off-by-one error in the line number for an error message. - When a second start marker is found without an intervening end marker, after giving an error, treat it as though it ends the section. It's hard to know exactly what to do in this case, but it makes unit testing this case a little simpler (in the next commit). - If an end marker occurs without a preceding start marker, issue an error.

This requires introducing `tidy_error_ext!` as an alternative to `tidy_error!`. It is passed a closure that is called for an error. This lets tests capture tidy error messages in a buffer instead of printing them to stderr. It also requires pulling part of `check` out into a new function `check_lines`, so that tests can pass in an iterator over some strings instead of a file.

nnethercote · 2023-10-25T00:12:54Z

Note: the first two commits are shared with #117068. Whichever PR lands first, the other can be updated afterwards.

cc @klensy

Noratrieb · 2023-10-28T16:05:45Z

@bors r+
thanks!

bors · 2023-10-28T16:05:47Z

📌 Commit a0b0ace has been approved by Nilstrieb

It is now in the queue for this repository.

bors · 2023-10-28T21:34:31Z

⌛ Testing commit a0b0ace with merge e5cfc55...

bors · 2023-10-28T23:35:38Z

☀️ Test successful - checks-actions
Approved by: Nilstrieb
Pushing e5cfc55 to master...

rust-timer · 2023-10-29T01:50:52Z

Finished benchmarking commit (e5cfc55): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	6.7%	[6.7%, 6.7%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 635.482s -> 635.311s (-0.03%)
Artifact size: 304.41 MiB -> 304.40 MiB (-0.00%)

nnethercote added 4 commits October 25, 2023 07:56

tidy: skip lines starting with # in alphabetical check.

40797ee

These are comment lines in `Cargo.toml` files. But exclude lines starting with `#!` from the skipping, because we want to check them. (Rust `#![feature(...)]` lines.) Also allow empty lines, which are occasionally useful.

rustbot assigned Noratrieb Oct 25, 2023

rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Oct 25, 2023

Noratrieb approved these changes Oct 28, 2023

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 28, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 28, 2023

bors merged commit e5cfc55 into rust-lang:master Oct 28, 2023
12 checks passed

rustbot added this to the 1.75.0 milestone Oct 28, 2023

nnethercote deleted the tidy-alphabetical-unit-tests branch October 29, 2023 21:24

nnethercote mentioned this pull request Oct 29, 2023

Clean up compiler/rustc*/Cargo.toml #117068

Merged

github-actions bot mentioned this pull request Oct 31, 2023

Automated pull from upstream master ferrocene/ferrocene#74

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tidy: add unit tests for alphabetical checks #117149

tidy: add unit tests for alphabetical checks #117149

nnethercote commented Oct 25, 2023

nnethercote commented Oct 25, 2023

Noratrieb commented Oct 28, 2023

bors commented Oct 28, 2023

bors commented Oct 28, 2023

bors commented Oct 28, 2023

rust-timer commented Oct 29, 2023

tidy: add unit tests for alphabetical checks #117149

tidy: add unit tests for alphabetical checks #117149

Conversation

nnethercote commented Oct 25, 2023

nnethercote commented Oct 25, 2023

Noratrieb commented Oct 28, 2023

bors commented Oct 28, 2023

bors commented Oct 28, 2023

bors commented Oct 28, 2023

rust-timer commented Oct 29, 2023

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size